Skip to content

Major error handling refactoring#101

Merged
mfateev merged 24 commits into
temporalio:masterfrom
mfateev:error_handling
Jun 17, 2020
Merged

Major error handling refactoring#101
mfateev merged 24 commits into
temporalio:masterfrom
mfateev:error_handling

Conversation

@mfateev
Copy link
Copy Markdown
Member

@mfateev mfateev commented Jun 16, 2020

Refactoring to the new Failure proto API.

This is to support propagation of chained errors across multiple SDKs. The biggest incompatible change from the application developer point of view that type of failure is returned as a cause of the ActivityFailure or WorkflowFailure instead of using child exceptions.

@mfateev mfateev requested a review from shawnhathaway June 16, 2020 05:14
this.nonRetryable = nonRetryable;
}

public boolean isNonRetryable() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there pros to this naming vs isRetryable? Seems off having a negative on a bool property

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pattern I always use is to have default value as the default value of the type. For boolean the default is false.

ActivitiesWorkflow[] workflows = new ActivitiesWorkflow[count];
WorkflowParams w = new WorkflowParams();
w.TemporalSleep = Duration.ofSeconds(1);
w.TemporalSleepMillis = 1000;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For defining time intervals - Do we prefer units on ints rather than strongly typed Duration?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer Duration unless it has to be passed across process boundary and serialized/deserialized. We can add serialization of Duration to the Jackson json serializer, but then it might not play well with Go and other SDKs.

return result;
}

public long getReplayTimeMillis() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed time types? ReplayTimeMillis vs Backoff

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReplayTimeMillis is a specific time of replay (timestamp). Backoff is a duration which is time interval.

List<String> types = new ArrayList<>();
// Use exception type name as the reason
List<Class<? extends Throwable>> doNotRetry = retryOptions.getDoNotRetry();
String[] doNotRetry = retryOptions.getDoNotRetry();
Copy link
Copy Markdown
Contributor

@shawnhathaway shawnhathaway Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for using string rather than types? Does previous usage cause the Illegal reflective access warnings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to be compatible with other SDKs. An activity can be implemented in Go and return an error type which is not Java exception name.

@mfateev mfateev merged commit e9361fc into temporalio:master Jun 17, 2020
@mfateev mfateev deleted the error_handling branch June 17, 2020 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants